[v24.x backport] src: do not enable wasm trap handler if there's not enough vmem#64338
Open
joyeecheung wants to merge 47 commits into
Open
[v24.x backport] src: do not enable wasm trap handler if there's not enough vmem#64338joyeecheung wants to merge 47 commits into
joyeecheung wants to merge 47 commits into
Conversation
Signed-off-by: Tim Perry <[email protected]> PR-URL: nodejs#63874 Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Signed-off-by: haramjeong <[email protected]> PR-URL: nodejs#63261 Fixes: nodejs#63258 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
CreateString() and Parse() in node_cjs_lexer.cc unconditionally called ToLocalChecked() on the results of String::NewFromOneByte(), String::NewFromUtf8() and Set::Add(). If string or handle allocation fails or an exception is pending on the isolate, these return an empty MaybeLocal and ToLocalChecked() aborts the process with "FATAL ERROR: v8::ToLocalChecked Empty MaybeLocal". Since Parse() is on the hot path of every ESM import of a CJS module (cjsPreparseModuleExports), propagate the failure as a regular pending JavaScript exception instead so callers can recover. Fixes: nodejs#63323 Refs: nodejs#61456 Signed-off-by: Yagiz Nizipli <[email protected]> PR-URL: nodejs#63885 Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: René <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Signed-off-by: Daijiro Wachi <[email protected]> PR-URL: nodejs#63815 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Jordan Harband <[email protected]>
Signed-off-by: Daijiro Wachi <[email protected]> PR-URL: nodejs#63883 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Deokjin Kim <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Ensure the debugger CLI waits for post-connect initialization before resolving the run and restart commands. This prevents tests and users from observing a prompt before Runtime.runIfWaitingForDebugger() has released the new debuggee. Refs: nodejs#61762 Signed-off-by: Kamat, Trivikram <[email protected]> Assisted-by: openai:gpt-5.5 PR-URL: nodejs#63607 Refs: nodejs#61762 Reviewed-By: Daijiro Wachi <[email protected]>
PR-URL: nodejs#63898 Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Signed-off-by: Daijiro Wachi <[email protected]> PR-URL: nodejs#63823 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Jordan Harband <[email protected]>
Signed-off-by: Daijiro Wachi <[email protected]> PR-URL: nodejs#63833 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Mattias Buelens <[email protected]>
PR-URL: nodejs#63899 Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Signed-off-by: Antoine du Hamel <[email protected]> Co-authored-by: Matteo Collina <[email protected]> PR-URL: nodejs#63909 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Gürgün Dayıoğlu <[email protected]> Reviewed-By: Mattias Buelens <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
The helper no longer treats every raw alias the same way and unintended values were accepted for some algorithms. Signed-off-by: Filip Skokan <[email protected]> PR-URL: nodejs#63910 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
When sqlite3_open_v2() fails, SQLite still assigns a database handle in a "sick" state. Such a handle may only be used to retrieve the error and must then be released with sqlite3_close(). Keeping it in connection_ meant that after a failed open(): - isOpen incorrectly reported true, - every method passed the "database is not open" check and called SQLite APIs on the sick handle, which is an API misuse, - registering a user-defined function leaked its user data in builds with SQLITE_ENABLE_API_ARMOR, because SQLite rejects the call before taking ownership of the user data, - open() could not be retried since the database appeared open. Close and reset the connection handle when open() fails at any point so that the database remains closed and open() can be retried. Fixes: nodejs#63831 Signed-off-by: Yagiz Nizipli <[email protected]> PR-URL: nodejs#63854 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Edy Silva <[email protected]>
The CJS mock.timers.setTime example asserted the timer ran right after setTime() with no tick() call, which contradicts the documented behavior and the ESM example above it. The mock.module example called .fn() on an export named foo. Both threw if run. Signed-off-by: Emmanuel Yusufu Kimaswa <[email protected]> PR-URL: nodejs#63656 Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Jacob Smith <[email protected]>
Signed-off-by: Filip Skokan <[email protected]> PR-URL: nodejs#63924 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Add micro-benchmarks that isolate the hot paths targeted by the JavaScript-to-C++ migration of child_process: - child-process-spawn-options.js scales the env vars and args that ProcessWrap::Spawn must marshal across the JS/C++ boundary. - child-process-ipc-roundtrip.js measures IPC throughput for both the json and advanced serializers across a range of payload sizes. - child-process-exec-maxbuffer.js measures stdout accumulation and maxBuffer handling in execFile(). These establish the baseline that later migration PRs are compared against. There is no runtime behavior change. Signed-off-by: Yagiz Nizipli <[email protected]> PR-URL: nodejs#63929 Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Signed-off-by: Antoine du Hamel <[email protected]> PR-URL: nodejs#63822 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Tim Perry <[email protected]>
Keep the initial --inspect-brk pause held until probe breakpoints are bound and probe mode explicitly releases the target. This prevents the generic pause handler from resuming user code before probes are ready. Refs: https://github.com/nodejs/node/actions/runs/26482141780/job/77981519238 Signed-off-by: Kamat, Trivikram <[email protected]> Assisted-by: openai:gpt-5.5 PR-URL: nodejs#63608 Refs: https://github.com/nodejs/node/actions/runs/26482141780/job/77981519238 Reviewed-By: Daijiro Wachi <[email protected]>
Signed-off-by: Matteo Collina <[email protected]> PR-URL: nodejs#63939 Fixes: nodejs/security-wg#1568 Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
The `advanced` IPC serialization codec was implemented in JavaScript (ChildProcessSerializer / ChildProcessDeserializer in lib/internal/child_process/serialization.js). It allocated a wrapper serializer/deserializer per message and crossed the JS/C++ boundary several times for every message (writeHeader, writeValue, releaseBuffer, readHeader, readValue and friends). Move the codec into a native `ipc_serdes` binding that drives the V8 ValueSerializer/ValueDeserializer with a C++ delegate. The wire format is preserved byte-for-byte: a big-endian uint32 length prefix followed by the V8 payload, with ArrayBufferViews tagged as host objects so that Node Buffers round-trip as Buffers rather than plain Uint8Arrays. The JSON codec is left unchanged. A cctest (test/cctest/test_node_ipc_serdes.cc) exercises the binding directly, covering round-trips of primitives, objects, typed arrays and Buffers (including the Buffer-vs-Uint8Array distinction) and asserting the big-endian length-prefix framing. Round-trip throughput (benchmark/child_process/child-process-ipc-roundtrip): payload before after change 64 B ~300k/s ~800k/s +166% 1 KiB ~272k/s ~616k/s +126% 16 KiB ~91k/s ~120k/s +32% 64 KiB ~30k/s ~35k/s +16% The gain is largest for small messages, where per-message JavaScript overhead dominated, and tapers for large messages, where the actual serialization (already native) dominates. Signed-off-by: Yagiz Nizipli <[email protected]> PR-URL: nodejs#63933 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
Closes: nodejs#59493 Signed-off-by: Filip Skokan <[email protected]> PR-URL: nodejs#63942 Fixes: nodejs#59493 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]>
Signed-off-by: Antoine du Hamel <[email protected]> PR-URL: nodejs#63819 Reviewed-By: Paolo Insogna <[email protected]>
This notable includes stream/iter (since they're closely related and rapidly evolving together) and ngtcp2, nghttp3 & openssl deps. Based on test-linux.yml, the main differences are simplifications: PRs only, filtered to specific paths, just one build (ARM only), no unusual characters test. Signed-off-by: Tim Perry <[email protected]> PR-URL: nodejs#63875 Reviewed-By: Stephen Belanger <[email protected]>
This changes `BignumPointer::GetWord` such that it does not hide errors from the caller. In the context of RSA keys within X.509 certificates, we should eventually compute the public exponent correctly regardless of its size. This patch, however, is designed to be a minimal change that prevents callers from using erroneous return values of `BN_get_word`. Signed-off-by: Tobias Nießen <[email protected]> PR-URL: nodejs#63895 Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]>
Signed-off-by: Tim Perry <[email protected]> PR-URL: nodejs#62217 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]>
This changes enables compression within OpenSSL *without* enabling record compression, so this only affects compression of certificates delivered within the TLS handshake. This certificate compression remains disabled by default for now, but becomes available via the new certificateCompression option in TLS context APIs. Enabling this shrinks handshakes significantly, and also reduces fingerprintability of Node.js client handshakes, as these are enabled in all modern browsers by default. Signed-off-by: Tim Perry <[email protected]> PR-URL: nodejs#62217 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]>
The `inspector.close()` forcefully terminates all active connections, allowing the process to exit cleanly, without waiting for the devtool client to disconnect. Signed-off-by: Chengzhong Wu <[email protected]> PR-URL: nodejs#63837 Reviewed-By: James M Snell <[email protected]>
Signed-off-by: Filip Skokan <[email protected]> PR-URL: nodejs#64009 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Tim Perry <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
Signed-off-by: Daijiro Wachi <[email protected]> PR-URL: nodejs#63964 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Signed-off-by: Daijiro Wachi <[email protected]> PR-URL: nodejs#63961 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
PR-URL: nodejs#62132 Refs: microsoft/vscode#251777 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/7638233 Reviewed-By: Aditi Singh <[email protected]> Signed-off-by: Joyee Cheung <[email protected]>
Collaborator
|
Review requested:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v24.x-staging #64338 +/- ##
=================================================
- Coverage 89.92% 89.87% -0.05%
=================================================
Files 686 694 +8
Lines 208389 212085 +3696
Branches 40077 40686 +609
=================================================
+ Hits 187387 190616 +3229
- Misses 13238 13578 +340
- Partials 7764 7891 +127
🚀 New features to boost your workflow:
|
Collaborator
Collaborator
Member
Author
|
CI is green. @nodejs/releasers can we include this in the next v24 release please? This would help address a class of bugs that basically manifests as "vscode remote ssh mysteriously getting stuck when connecting to a server". |
richardlau
approved these changes
Jul 14, 2026
joyeecheung
force-pushed
the
backport-62132-to-24
branch
from
July 17, 2026 21:34
0eeb4f5 to
3048bdc
Compare
joyeecheung
added a commit
to joyeecheung/node
that referenced
this pull request
Jul 17, 2026
Original commit message:
[api] Add V8::GetWasmMemoryReservationSizeInBytes()
When the system does not have enough virtual memory for the wasm
cage, installing the trap handler would cause any code allocating
wasm memory to throw. Therefore it's useful for the embedder to
know when the system doesn't have enough virtual address space
to allocate enough wasm cages and in that case, skip the
trap handler installation so that wasm code can at least work
(even not at the maximal performance).
Node.js previously has a command line option
--disable-wasm-trap-handler to fully disable trap-based bound checks,
this new API would allow it to adapt automatically while keeping the
optimization in the happy path, since it's not always possible for
end users to opt-into disabling trap-based bound checks (for example,
when a VS Code Server is loaded in a remote server for debugging).
Refs: nodejs#62132
Refs: microsoft/vscode#251777
Change-Id: I345c076af2b2b47700e5716b49c3133fdf8a0981
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Reviewed-by: Jakob Kummerow <[email protected]>
Reviewed-by: Michael Lippautz <[email protected]>
Commit-Queue: Joyee Cheung <[email protected]>
Reviewed-by: Clemens Backes <[email protected]>
Cr-Commit-Position: refs/heads/main@{#105702}
Refs: v8/v8@bef0d9c
Co-authored-by: Joyee Cheung <[email protected]>
Refs: microsoft/vscode#251777
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Signed-off-by: Joyee Cheung <[email protected]>
PR-URL: nodejs#64338
Reviewed-By: Richard Lau <[email protected]>
joyeecheung
added a commit
to joyeecheung/node
that referenced
this pull request
Jul 17, 2026
Refs: microsoft/vscode#251777 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/7638233 Signed-off-by: Joyee Cheung <[email protected]> PR-URL: nodejs#64338 Reviewed-By: Richard Lau <[email protected]>
joyeecheung
added a commit
to joyeecheung/node
that referenced
this pull request
Jul 17, 2026
Original commit message:
[api] Add V8::GetWasmMemoryReservationSizeInBytes()
When the system does not have enough virtual memory for the wasm
cage, installing the trap handler would cause any code allocating
wasm memory to throw. Therefore it's useful for the embedder to
know when the system doesn't have enough virtual address space
to allocate enough wasm cages and in that case, skip the
trap handler installation so that wasm code can at least work
(even not at the maximal performance).
Node.js previously has a command line option
--disable-wasm-trap-handler to fully disable trap-based bound checks,
this new API would allow it to adapt automatically while keeping the
optimization in the happy path, since it's not always possible for
end users to opt-into disabling trap-based bound checks (for example,
when a VS Code Server is loaded in a remote server for debugging).
Refs: nodejs#62132
Refs: microsoft/vscode#251777
Change-Id: I345c076af2b2b47700e5716b49c3133fdf8a0981
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Reviewed-by: Jakob Kummerow <[email protected]>
Reviewed-by: Michael Lippautz <[email protected]>
Commit-Queue: Joyee Cheung <[email protected]>
Reviewed-by: Clemens Backes <[email protected]>
Cr-Commit-Position: refs/heads/main@{#105702}
Refs: v8/v8@bef0d9c
Co-authored-by: Joyee Cheung <[email protected]>
Refs: microsoft/vscode#251777
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Signed-off-by: Joyee Cheung <[email protected]>
PR-URL: nodejs#64338
Reviewed-By: Richard Lau <[email protected]>
joyeecheung
added a commit
to joyeecheung/node
that referenced
this pull request
Jul 17, 2026
Refs: microsoft/vscode#251777 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/7638233 Signed-off-by: Joyee Cheung <[email protected]> PR-URL: nodejs#64338 Reviewed-By: Richard Lau <[email protected]>
joyeecheung
force-pushed
the
backport-62132-to-24
branch
from
July 17, 2026 21:38
3048bdc to
5159d50
Compare
Member
Author
|
CI was green. Force pushed to sign the commits, but it seems I cannot land it myself - maybe it needs to be someone from @nodejs/releasers ? |
aduh95
force-pushed
the
backport-62132-to-24
branch
from
July 21, 2026 14:47
5159d50 to
0eeb4f5
Compare
aduh95
pushed a commit
that referenced
this pull request
Jul 21, 2026
Original commit message:
[api] Add V8::GetWasmMemoryReservationSizeInBytes()
When the system does not have enough virtual memory for the wasm
cage, installing the trap handler would cause any code allocating
wasm memory to throw. Therefore it's useful for the embedder to
know when the system doesn't have enough virtual address space
to allocate enough wasm cages and in that case, skip the
trap handler installation so that wasm code can at least work
(even not at the maximal performance).
Node.js previously has a command line option
--disable-wasm-trap-handler to fully disable trap-based bound checks,
this new API would allow it to adapt automatically while keeping the
optimization in the happy path, since it's not always possible for
end users to opt-into disabling trap-based bound checks (for example,
when a VS Code Server is loaded in a remote server for debugging).
Refs: #62132
Refs: microsoft/vscode#251777
Change-Id: I345c076af2b2b47700e5716b49c3133fdf8a0981
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Reviewed-by: Jakob Kummerow <[email protected]>
Reviewed-by: Michael Lippautz <[email protected]>
Commit-Queue: Joyee Cheung <[email protected]>
Reviewed-by: Clemens Backes <[email protected]>
Cr-Commit-Position: refs/heads/main@{#105702}
Refs: v8/v8@bef0d9c
Co-authored-by: Joyee Cheung <[email protected]>
PR-URL: #62132
Backport-PR-URL: #64338
Refs: microsoft/vscode#251777
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Signed-off-by: Joyee Cheung <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
aduh95
pushed a commit
that referenced
this pull request
Jul 21, 2026
PR-URL: #62132 Backport-PR-URL: #64338 Refs: microsoft/vscode#251777 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/7638233 Signed-off-by: Joyee Cheung <[email protected]> Reviewed-By: Richard Lau <[email protected]>
aduh95
pushed a commit
that referenced
this pull request
Jul 22, 2026
Original commit message:
[api] Add V8::GetWasmMemoryReservationSizeInBytes()
When the system does not have enough virtual memory for the wasm
cage, installing the trap handler would cause any code allocating
wasm memory to throw. Therefore it's useful for the embedder to
know when the system doesn't have enough virtual address space
to allocate enough wasm cages and in that case, skip the
trap handler installation so that wasm code can at least work
(even not at the maximal performance).
Node.js previously has a command line option
--disable-wasm-trap-handler to fully disable trap-based bound checks,
this new API would allow it to adapt automatically while keeping the
optimization in the happy path, since it's not always possible for
end users to opt-into disabling trap-based bound checks (for example,
when a VS Code Server is loaded in a remote server for debugging).
Refs: #62132
Refs: microsoft/vscode#251777
Change-Id: I345c076af2b2b47700e5716b49c3133fdf8a0981
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Reviewed-by: Jakob Kummerow <[email protected]>
Reviewed-by: Michael Lippautz <[email protected]>
Commit-Queue: Joyee Cheung <[email protected]>
Reviewed-by: Clemens Backes <[email protected]>
Cr-Commit-Position: refs/heads/main@{#105702}
Refs: v8/v8@bef0d9c
Co-authored-by: Joyee Cheung <[email protected]>
PR-URL: #62132
Backport-PR-URL: #64338
Refs: microsoft/vscode#251777
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/7638233
Signed-off-by: Joyee Cheung <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
aduh95
pushed a commit
that referenced
this pull request
Jul 22, 2026
PR-URL: #62132 Backport-PR-URL: #64338 Refs: microsoft/vscode#251777 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/7638233 Signed-off-by: Joyee Cheung <[email protected]> Reviewed-By: Richard Lau <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a backport of #62132 which would help closing microsoft/vscode#251777